Skip to content

Conversation

@pquentin
Copy link
Member

@pquentin pquentin commented Sep 9, 2025

Many bodies were correctly set as optional in the rest-api-spec, but incorrectly set as required in the Elasticsearch specification. There are three errors in rest-api-spec too. When I fix them, I can commit the compiler change that allowed me to see this.

@pquentin pquentin requested review from a team as code owners September 9, 2025 13:09
@github-actions
Copy link
Contributor

github-actions bot commented Sep 9, 2025

Following you can find the validation changes against the target branch for the API.

API Status Request Response
indices.simulate_index_template 🔴 8/10 → 10/10 6/10

You can validate this API yourself by using the make validate target.

Copy link
Member Author

@pquentin pquentin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For each change, I have linked the corresponding server code justifying the change and sometimes additional evidence.

master_timeout?: Duration
}
body: {
body?: {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sort?: string | string[]
}
body: {
body?: {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

master_timeout?: Duration
}
body: {
body?: {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

timeout?: Duration
}
body: {
body?: {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines +68 to +69
/** @codegen_name index_template */
body?: IndexTemplate
Copy link
Member Author

debug?: boolean
}
body: {
body?: {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
]
body: {
body?: {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

id?: Id
}
body: {
body?: {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

include_defaults?: boolean
}
body: {
body?: {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
/** @codegen_name create_from */
body: CreateFrom
body?: CreateFrom
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

@qn895 qn895 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ML & Transform changes LGTM

Copy link
Member Author

@pquentin pquentin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added two APIs which I initially thought required a body. The server code was unclear, but the YAML tests provided clarity.

wait_for_completion?: boolean
}
body: {
body?: {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait_for_completion?: boolean
}
body: {
body?: {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@l-trotta l-trotta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, and thanks for the detailed comments @pquentin!

@pquentin pquentin merged commit 3348b2f into main Sep 10, 2025
10 checks passed
@pquentin pquentin deleted the fix-optional-bodies branch September 10, 2025 11:55
github-actions bot pushed a commit that referenced this pull request Sep 10, 2025
* Fix optional bodies

* Mark snapshot.create/snapshot.restore bodies as optional too

(cherry picked from commit 3348b2f)
github-actions bot pushed a commit that referenced this pull request Sep 10, 2025
* Fix optional bodies

* Mark snapshot.create/snapshot.restore bodies as optional too

(cherry picked from commit 3348b2f)
pquentin added a commit that referenced this pull request Sep 10, 2025
* Fix optional bodies

* Mark snapshot.create/snapshot.restore bodies as optional too

(cherry picked from commit 3348b2f)

Co-authored-by: Quentin Pradet <[email protected]>
pquentin added a commit that referenced this pull request Sep 10, 2025
* Fix optional bodies

* Mark snapshot.create/snapshot.restore bodies as optional too

(cherry picked from commit 3348b2f)

Co-authored-by: Quentin Pradet <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants